Skip to content

Conversation

heyrandhir
Copy link
Contributor

  • Update api contracts for /users/filter.
  • with this API Users can be filtered based on their skills and status i.e idle, active, or OOO.

sahsisunny
sahsisunny previously approved these changes Apr 3, 2023
Copy link
Contributor

@sahsisunny sahsisunny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

| [GET /users/:userId/badges](#get-usersidbadges) | Returns badges assigned to the user |
| [POST /users](#post-users) | Creates a new User |
| [PATCH /users/self](#patch-usersself) | Updates data of the User |
| [GET /users/filter](#get-usersfilter) | Returns user data based on a filter |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ?filter. (Since filter is not a resource)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In another issue here, /users/userId is being modified to /users?userId=.. .As this change also accepts query parameters directly after /users. Adding query parameters here as well after /users will cause a collision.

can we rename this endpoint to /users/search?....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heyrandhir are we going with /users/filter or /users/search for this API?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vikhyat187 as of now going with /users/search .also waiting for @ankushdharkar comment on this one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just give the results based on the key provided in the query parameters?

If asked for the filter, we filter and give, if asked for ?userId, we give user details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants